-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't include objc_library generated module maps in packaged frameworks #180
Don't include objc_library generated module maps in packaged frameworks #180
Conversation
@@ -172,6 +172,10 @@ def _apple_framework_packaging_impl(ctx): | |||
|
|||
# collect modulemaps | |||
for modulemap in dep[apple_common.Objc].direct_module_maps: | |||
if modulemap.owner == dep.label: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to make sure I understand this correctly, for direct module maps of a given obj-c dependency, if the module map is generated by the dep itself, including it will cause redefinition error, so skip them at this point is the way to fix this error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Could we expand the test setup to cover more complex situations i.e. Unit Tests with bridging headers that import headers from the tested target? I have made the changes in this commit 1bd0996 |
Actually I can break this when using a bridging header with unit tests, see changes here #181 |
6913263
to
9f3fa62
Compare
Looping back, my import style in my bridging header was breaking this. It would be great if we could also merge in the test updates here #181 |
Fixes #179, by no longer leaking an incorrect module map